Add settleAndRefund
#2
Conversation
ChefMist
requested review from
ChefSnoopy,
0xjojoex,
chef-omelette and
chefburger
|
rest lgtm |
src/Vault.sol
Outdated
| isLocked | ||
| returns (uint256 paid, uint256 refund) | ||
| { | ||
| paid = settle(currency); |
There was a problem hiding this comment.
Should we calculate the amount we need to refund in advance? then just need to execute SettlementGuard.accountDelta once if have excess token.
function settleAndRefund(Currency currency, address to)
external
payable
override
isLocked
returns (uint256 paid, uint256 refund)
{
paid = currency.balanceOfSelf() - reservesOfVault[currency];
// need to consider the case when the currencyDelta is negative ?.
uint256 currentCurrencyDelta = (SettlementGuard.getCurrencyDelta(msg.sender, currency)).toUint256();
if (paid > currentCurrencyDelta) {
refund = paid - currentCurrencyDelta;
paid = currentCurrencyDelta;
}
reservesOfVault[currency] += paid;
SettlementGuard.accountDelta(msg.sender, currency, -(paid.toInt128()));
if (refund > 0) {
currency.transfer(to, refund);
}
}
There was a problem hiding this comment.
thanks! adopted this idea since it'll save 1 sload for refund case. though did a small tweak to handle negative currency delta as settle() wont revert with SafeCastOverflow for negative balance delta
| @@ -0,0 +1 @@ | |||
| 56136 | |||
There was a problem hiding this comment.
was 79085 gas previously and now dip to 56136 with Snoopy's implementation
src/Vault.sol
Outdated
| // msg.sender owes vault but paid more than than whats owed | ||
| refund = paid - currentDelta.toUint256(); | ||
| paid = currentDelta.toUint256(); | ||
| currency.transfer(to, refund); |
There was a problem hiding this comment.
We need to think about whether there are re-entrant issues.
Let me think about this. maybe need to put this in the end after updated reservesOfVault and SettlementGuard.
Description
In normal cases, user would transfer the token to the vault and call
vault.settle()eg.However, someone can potentially cause the txn to fail with
CurrencyNotSettled()by sending 1 wei of token0 or token1 into the vault.Fix
This PR adds
settleAndRefundwhich handle this cases. It will be used by the v4-periphery contracts and should help to remove excess tokens in vault.